refactor(core): separate persisted decode boundaries - #3562

Reviewed at exact head 35344e23bda0bd2615c36677cd31299e2767d074.

Coverage of this review: behavioural equivalence of the persisted-decode split (old data read through the new path), error-propagation paths, and reachability of the removed code. Not covered: naming/file organisation, UI.

Result: no P0–P2 findings in the covered scope.

What was checked

• AgentRun header. The old path validated shape, then folded status / permissionMode, then validated. The new path splits this into a persisted fold (waiting_permission → waiting_for_user map, decodePersistedPermissionMode) followed by a strict canonical validation (isPermissionMode). Accept/reject outcomes were compared field by field for retired spellings, unknown junk, and missing fields — they match.
• Tool-result content. Subagent permissionMode folding moved out of the canonical decoder into the persisted variant. The folding scope is unchanged (subagent only).
• Call sites. Every persisted read now uses the persisted variant with markPersisted (conversation-copy ×3, read-model, session-store/sqlite metadata read+write, Desktop/CLI loadTranscript). Write paths and protocol frames stay canonical, which is correct for in-process domain data. subagent-tools / tool-runtime construct fresh in-process content, so canonical is right there too.
• Removed code. foldRetiredPermissionMode has no remaining callers. The continuation-claim persisted read at packages/storage/src/sqlite-runtime-store.ts:3723 correctly gained decodePersistedAgentRunHeader, so the strict decode inside claim receives an already-folded header — the easiest one to miss in this refactor, and it is not missed.

Two deliberate tightenings, both fail-closed

Unknown effect.kind, agent_run missing execution, and an undecodable permissionMode in scheduled tasks now throw where the old code passed them through to the execution layer. The effect union only has notify / session_resume / agent_run, so valid historical data is unaffected. Closing the "accept a corrupt record as valid" door is the right direction.

[P3] Batch decode failure has no bad-row skip

packages/storage/src/scheduled-task-store.ts:598-621 — in the list/claim batch read, one row failing to decode throws for the whole batch (previously an unknown kind passed silently). This is consistent with the existing semantics, since JSON-layer corruption already fails the whole batch, and it is not a regression introduced here. Recording it only as an operational-resilience observation: if a single bad row ever appears, the entire task list becomes unreadable with no degraded path that skips it. No change requested.

Verification

Targeted suites run locally against the exact head after building: core 48/48 (including the new persisted-value-contract tests), storage 121/121, runtime conversation-copy 11/11 and read-model 57/57, runtime-host 32/32 — 269 green.

Limits, stated plainly:

• Hosted CI has not run on this head — check-runs total_count is 0 and the combined status is pending with zero statuses. That is neither green nor red, so this review is a comment only and does not clear the CI gate.
• Equivalence is established by source-level comparison plus targeted tests. No real legacy database file was constructed for an end-to-end migration test; retired-spelling coverage rests on the old and new unit-test assertions.

Follow-up to my earlier comment: hosted CI had never run on this head, so I triggered it. It has now run and test fails — for a reason unrelated to anything in my previous review.

Protocol epoch guard: Protocol files changed but RUNTIME_HOST_COMPATIBILITY_EPOCH is still 39,
the current base parent's value.
Changed files without a compatible-change declaration:
  packages/runtime-host/src/protocol/session-turns.ts

The guard's own message states the remedy: same-number bumps on sibling branches merge without a git conflict (#3313), so a protocol change has to land with an epoch the current base has not seen. Rebase onto current main and set RUNTIME_HOST_COMPATIBILITY_EPOCH past 39 — or, if you consider the session-turns.ts change compatible, add the compatible-change declaration the guard is looking for.

My review findings are unchanged: still no P0–P2 in the covered scope, with the one [P3] on batch decode failure. This is purely the CI gate that was previously invisible because no run existed.

Reviewed on exact head b430e200b0c879697c55cefeaa14d62a7f7da1ec. No P0–P2. Two design-level [P3] notes inline, neither blocking.

The question this refactor lives or dies on is backward compatibility: if a decoder gets stricter on a path that reads durable rows, old Sessions stop opening. The deleted comment in session-store.ts said exactly that — "rejecting it would make the Session unopenable".

That risk is contained, and the containment was checked by enumeration rather than sampling:

• PersistedValue<T> is a compile-time phantom brand; markPersisted adds no runtime wrapper. So the only way this PR can break old data is a decoder becoming stricter on a read path.
• Two decoders did get stricter, both deliberately: hasValidSubagentResultFields in tool-result-record-schema.ts and normalizeSessionHeader in session-store.ts. Folding moved out of the canonical decoders into new persisted-path decoders.
• All 8 non-test call sites of normalizeSessionHeader were enumerated. sqlite-session-metadata-store.ts:743/819/887/938/1338 take a SessionHeader method parameter (new input); :3893/:3917 operate on already-decoded values; project-catalog.ts:757 receives the output of decodePersistedSessionHeader at :746. None of them consumes a raw persisted row. The only funnels for raw rows are sqlite-session-metadata-store.ts:5284 and project-catalog.ts:746, and both go through the tolerant persisted decoder.
• Same on the message side: the strict decodeCanonicalMessage has non-test call sites only at session-turns.ts:409 (wire boundary) and three write-path JSON round-trips. Reads go through the tolerant decodeStoredMessage.

One nuance worth recording, because the epoch 42 note says this lands "without changing the accepted Session turn contribution shape": strictly the accepted shape does narrow — a subagent tool result carrying a retired permission mode would now be rejected at that boundary. It is unreachable in production because the Host normalizes before the value ever goes on the wire (readTurnContributions → decodeStoredMessageRecordRow → decodeStoredMessage, which folds execute → ask), and older Hosts folded in the same place. So the claim is accurate in effect, just not literally. RETIRED_PERMISSION_MODES currently holds one entry.

On gating: dependency-audit.yml lists packages/*/package.json in its pull_request.paths, and this PR adds a ./persisted-value export to packages/core/package.json, so audit genuinely is an expected check here. It had been parked at action_required with zero check-runs, which reads as "absent" rather than "not green". That run has since been released and audit is now terminal success, alongside test. Both expected checks are green on this exact head.

Two inline [P3] design notes, split out from the approval so they are easy to track separately. Neither blocks this PR.